Skip to content

Conversation

@sgny
Copy link
Contributor

@sgny sgny commented Oct 6, 2019

Here is what I pushed to the repo (and undid later).

I suppose once @tatchi's bindings are merged, we can pull documentation, etc. from this.

@sgny sgny changed the title initial release add documentation Oct 11, 2019
@sgny
Copy link
Contributor Author

sgny commented Oct 11, 2019

I merged @tatchi's version as he had already contributed the important part. This revised PR adds documentation and brings this repo in line with the template.

However, some remaining issues are

  • resolving @reason-react-native/react-native vs reason-react-native
  • whether we refer to the module as ReactNativeCheckBox or ReactNativeCheckbox
  • use open ReactNative or not

In the interim, I commited a bug fix for tintColors.

Copy link
Member

@MoOx MoOx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Just a tiny thing to remove :D

@MoOx
Copy link
Member

MoOx commented Oct 14, 2019

@sgny To answer you

  • Use reason-react-native
  • I would go with "Checkbox" as it's the common spelling (even wikipedia know this)
  • open ReactNative if that's required, it's not an issue as we are doing this everywhere.

@sgny
Copy link
Contributor Author

sgny commented Oct 14, 2019

Will implement those

One issue, the component is addressed from JS as CheckBox. That's why I figured if we should have it spelled like that.

@sgny
Copy link
Contributor Author

sgny commented Oct 19, 2019

@MoOx merging this, the only open issue is the name for the module, which is fine as it is.

@sgny sgny merged commit d20b42b into master Oct 19, 2019
@sgny sgny deleted the withdocs branch October 19, 2019 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants